Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New tests for human readable values #1623

Merged
merged 8 commits into from
Sep 30, 2015
Merged

New tests for human readable values #1623

merged 8 commits into from
Sep 30, 2015

Conversation

pkess
Copy link
Contributor

@pkess pkess commented Sep 27, 2015

Here i added some tests for the output of human readable values.
Especially the test of human_bytes is written as it would success now. But is the implementation really correct?
As you see at https://en.wikipedia.org/wiki/Petabyte a Petabyte is 10^15 bytes and not 2^50 bytes as implemented.

@sampsyo
Copy link
Member

sampsyo commented Sep 28, 2015

Great! But yes indeed—it looks like the exponents in the original code are wrong. 😢 Would you mind fixing them too?

@pkess
Copy link
Contributor Author

pkess commented Sep 28, 2015

I will be happy to fix it, but which style do we want to use?
10^n xB
or 2^n xiB

@sampsyo
Copy link
Member

sampsyo commented Sep 28, 2015

Good question. Let's say XiB (i.e., powers of two) since that's closest to what we're doing (or thought we were doing) already.

@pkess
Copy link
Contributor Author

pkess commented Sep 30, 2015

So i think i fixed it now.
Feel free to merge it

@sampsyo
Copy link
Member

sampsyo commented Sep 30, 2015

Awesome. ✨ I'll hit the merge button now, but would you mind also adding a note to the changelog about the fix?

sampsyo added a commit that referenced this pull request Sep 30, 2015
New tests for human readable values
@sampsyo sampsyo merged commit 092472d into beetbox:master Sep 30, 2015
@pkess pkess deleted the test_init branch September 30, 2015 17:42
pkess added a commit to pkess/beets that referenced this pull request Sep 30, 2015
pkess added a commit to pkess/beets that referenced this pull request Sep 30, 2015
pkess added a commit to pkess/beets that referenced this pull request Sep 30, 2015
@pkess
Copy link
Contributor Author

pkess commented Sep 30, 2015

Done. Hope you enjoy it

@sampsyo
Copy link
Member

sampsyo commented Sep 30, 2015

Looks perfect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants